本篇同步發布於個人Blog: [Refactoring] Chapter 1 Refactoring: A First Example - RPG Game Hunting Mission
I will write a series of refactoring article based on Martin Fowler's Refactoring: Improving the Design of Existing Code (2nd Edition). This book introduced many useful refactoring techniques. So I want to share what I learned from this book by explaining some examples and noting important concepts/tips.
In the chapter 1 of the book, it introduced a case about the bill calculation of performing theatrical plays. The case first was an ugly function code to generate a bill. After some refactoring skills, the code looked CLEAR. How beautiful the code is!
According to this case, I will mock a similar function code to show how the refactoring skill works. The following code is written by C# & .NET 5.
This scenario is a RPG Game. In a RPG Game, a player can take hunting mission to earn bonus and experience. If player completes the mission, then mission manager will give you the bonus/experience. The type and level of monster killed by player determines the amount of bonus and experience.
Bless Unleashed (Picture Source: https://www.mmorpg.com/news/bless-unleashed-gm-battlefield-event-for-june-19-2000122222)
Mission manager have a monster list that shows the id, name and type. As the following Figure 1 shows:
Figure 1. Monster List (Monster picture source: https://opengameart.org/content/dungeon-crawl-selected-upscale)
One hunting mission includes these monsters as the following Figure 2 shows:
Figure 2. Mission Detail
I create some classes representing the structure of these monster list and mission.
glj8989332
public class HuntingMission
{
public string Name { get; }
public List<HuntingTarget> Targets { get; }
public HuntingMission(string name, List<HuntingTarget> targets)
{
Name = name;
Targets = targets;
}
}
public class HuntingTarget
{
public string MonsterId { get; }
public int Level { get; }
public HuntingTarget(string monsterId, int level)
{
MonsterId = monsterId;
Level = level;
}
}
public class Monster
{
public string Name { get; }
public string Type { get; }
public Monster(string name, string type)
{
Name = name;
Type = type;
}
}
Then create instances representing the above monster list and mission:
Dictionary<string, Monster> monsters = new Dictionary<string, Monster>();
monsters.Add("fire_giant", new Monster("Fire Giant", "giant"));
monsters.Add("stone_giant", new Monster("Stone Giant", "giant"));
monsters.Add("bone_dragon", new Monster("Bone Dragon", "dragon"));
HuntingMission mission = new HuntingMission("Giant & Dragon Hunting", new List<HuntingTarget>(new[]
{
new HuntingTarget("fire_giant", 55),
new HuntingTarget("bone_dragon", 40),
new HuntingTarget("stone_giant", 61)
}));
List<HuntingMission> missions = new List<HuntingMission>(new[] { mission });
Create a mission manager class and a function Statement. Pass the monster list and mission to this Statement function and it returns a calculated bonus/experience string.
public class MissionManager
{
public string Statement(HuntingMission mission, Dictionary<string, Monster> monsters)
{
int totalExperience = 0;
int totalBonus = 0;
string result = $"Statement for {mission.Name}\n";
CultureInfo enCulture = new CultureInfo("en-US");
Thread.CurrentThread.CurrentCulture = enCulture;
NumberFormatInfo localFormat = (NumberFormatInfo)NumberFormatInfo.CurrentInfo.Clone();
foreach (var missionTarget in mission.Targets)
{
var monster = monsters[missionTarget.MonsterId];
int thisBonus = 0;
switch (monster.Type)
{
case "giant":
thisBonus = 60000;
if (missionTarget.Level > 25)
{
thisBonus += 500 * (missionTarget.Level - 25);
}
break;
case "dragon":
thisBonus = 80000;
if (missionTarget.Level > 35)
{
thisBonus += 2500 + 900 * (missionTarget.Level - 35);
}
thisBonus += 200 * missionTarget.Level;
break;
default:
throw new Exception($"Unknown type: {monster.Type}");
}
// add experience
totalExperience += Math.Max(missionTarget.Level - 40, 0);
// add extra experience for every dragon monster by level dividing by 3
if (monster.Type == "dragon")
{
totalExperience += missionTarget.Level / 3;
}
result +=
$"{monster.Name}: {(thisBonus / 100).ToString("c", localFormat)} ({missionTarget.Level} levels)\n";
totalBonus += thisBonus;
}
result += $"Bonus was {(totalBonus / 100).ToString("c", localFormat)}\n";
result += $"You got {totalExperience} experiences\n";
return result;
}
}
The tip at book's page 4 describes:
_**
When you have to add a feature to a program but the code is not structured in a convenient way, first refactor the program to make it easy to add the feature, then add the feature.
**_
As this tip's guide, let's refactor the code!
The tip at book's page 5 describes:
**_
Before you start refactoring, make sure you have a solid suite of tests. These tests must be self-checking.
_**
I have created a unit testing project to check current's function is correct.
public class MIssionManagerTests
{
[Test]
public void Given_Mission_Monsters_Should_Return_Correct_Statement()
{
// arrange
Dictionary<string, Monster> monsters = new Dictionary<string, Monster>();
monsters.Add("fire_giant", new Monster("Fire Giant", "giant"));
monsters.Add("stone_giant", new Monster("Stone Giant", "giant"));
monsters.Add("bone_dragon", new Monster("Bone Dragon", "dragon"));
HuntingMission mission = new HuntingMission("Giant & Dragon Hunting", new List<HuntingTarget>(new[]
{
new HuntingTarget("fire_giant", 55),
new HuntingTarget("bone_dragon", 40),
new HuntingTarget("stone_giant", 61)
}));
List<HuntingMission> missions = new List<HuntingMission>(new[] { mission });
// act
string result = new MissionManager().Statement(missions.First(), monsters);
// assert
Assert.AreEqual(
"Statement for Giant & Dragon Hunting\nFire Giant: $750.00 (55 levels)\nBone Dragon: $950.00 (40 levels)\nStone Giant: $780.00 (61 levels)\nBonus was $2,480.00\nYou got 49 experiences\n",
result);
}
}
After call the Statement function with the above monster list and mission, the returned string is:
Statement for Giant & Dragon Hunting
Fire Giant: $750.00 (55 levels)
Bone Dragon: $950.00 (40 levels)
Stone Giant: $780.00 (61 levels)
Bonus was $2,480.00
You got 49 experiences
So the assertion is used to check the returned result is equal to the above string.
I will change some code and use the unit testing to check again that my code keeps original solution. Testings are very important!
The middle of the code has a switch statement that calculates the current bonus amount based on monster type. Let's do the Extract Function: the switch statement is extracted to an amountFor delegate function:
public class MissionManager
{
public string Statement(HuntingMission mission, Dictionary<string, Monster> monsters)
{
Func<HuntingTarget, Monster, int> amountFor = delegate (HuntingTarget missionTarget, Monster monster)
{
int thisBonus = 0;
switch (monster.Type)
{
case "giant":
thisBonus = 60000;
if (missionTarget.Level > 25)
{
thisBonus += 500 * (missionTarget.Level - 25);
}
break;
case "dragon":
thisBonus = 80000;
if (missionTarget.Level > 35)
{
thisBonus += 2500 + 900 * (missionTarget.Level - 35);
}
thisBonus += 200 * missionTarget.Level;
break;
default:
throw new Exception($"Unknown type: {monster.Type}");
}
return thisBonus;
};
int totalExperience = 0;
int totalBonus = 0;
string result = $"Statement for {mission.Name}\n";
CultureInfo enCulture = new CultureInfo("en-US");
Thread.CurrentThread.CurrentCulture = enCulture;
NumberFormatInfo localFormat = (NumberFormatInfo)NumberFormatInfo.CurrentInfo.Clone();
foreach (var missionTarget in mission.Targets)
{
var monster = monsters[missionTarget.MonsterId];
int thisBonus = amountFor(missionTarget, monster);
// add experience
totalExperience += Math.Max(missionTarget.Level - 40, 0);
// add extra experience for every dragon monster by level dividing by 3
if (monster.Type == "dragon")
{
totalExperience += missionTarget.Level / 3;
}
result +=
$"{monster.Name}: {(thisBonus / 100).ToString("c", localFormat)} ({missionTarget.Level} levels)\n";
totalBonus += thisBonus;
}
result += $"Bonus was {(totalBonus / 100).ToString("c", localFormat)}\n";
result += $"You got {totalExperience} experiences\n";
return result;
}
}
This amountFor delegate function is simulating the code block like book's JavaScript logic. If you are using C# 7.0 or higher version, it can be converted into local function.
After run the unit testing, this code keeps original solution. The missionTarget and monster parameters are not modified in switch statement, so they can be passed to a function.
The tip at book's page 8 describes:
_
Refactoring changes the programs in small steps, so if you make a mistake, it is easy to find where the bug is.
_
In the amountFor delegate function, the thisBonus variable naming is not clear, so change it to result naming:
Func<HuntingTarget, Monster, int> amountFor = delegate (HuntingTarget missionTarget, Monster monster)
{
int result = 0;
switch (monster.Type)
{
case "giant":
result = 60000;
if (missionTarget.Level > 25)
{
result += 500 * (missionTarget.Level - 25);
}
break;
case "dragon":
result = 80000;
if (missionTarget.Level > 35)
{
result += 2500 + 900 * (missionTarget.Level - 35);
}
result += 200 * missionTarget.Level;
break;
default:
throw new Exception($"Unknown type: {monster.Type}");
}
return result;
};
The result naming represents its responsibility and increases readability.
The tip at book's page 10 describes:
Any fool can write code that a computer can understand. Good programmers write code that humans can understand.
What a ABSOLUTE TRUE!
Let's check the amountFor function, missionTarget variable is set a new value by every for loop, but monster variable is calculated by missionTarget. So monster variable is not required to be passed as a parameter.
Use a refactoring method Replace Temp with Query: extracting the right-hand site of the assignment into a function.
public string Statement(HuntingMission mission, Dictionary<string, Monster> monsters)
{
Func<HuntingTarget, Monster> monsterFor = delegate(HuntingTarget missionTarget)
{
return monsters\[missionTarget.MonsterId\];
};
Func<HuntingTarget, Monster, int> amountFor = delegate (HuntingTarget missionTarget, Monster monster)
{
int result = 0;
switch (monster.Type)
{
case "giant":
result = 60000;
if (missionTarget.Level > 25)
{
result += 500 * (missionTarget.Level - 25);
}
break;
case "dragon":
result = 80000;
if (missionTarget.Level > 35)
{
result += 2500 + 900 * (missionTarget.Level - 35);
}
result += 200 * missionTarget.Level;
break;
default:
throw new Exception($"Unknown type: {monster.Type}");
}
return result;
};
int totalExperience = 0;
int totalBonus = 0;
string result = $"Statement for {mission.Name}\n";
CultureInfo enCulture = new CultureInfo("en-US");
Thread.CurrentThread.CurrentCulture = enCulture;
NumberFormatInfo localFormat = (NumberFormatInfo)NumberFormatInfo.CurrentInfo.Clone();
foreach (var missionTarget in mission.Targets)
{
var monster = monsterFor(missionTarget);
int thisBonus = amountFor(missionTarget, monster);
// add experience
totalExperience += Math.Max(missionTarget.Level - 40, 0);
// add extra experience for every dragon monster by level dividing by 3
if (monster.Type == "dragon")
{
totalExperience += missionTarget.Level / 3;
}
result +=
$"{monster.Name}: {(thisBonus / 100).ToString("c", localFormat)} ({missionTarget.Level} levels)\n";
totalBonus += thisBonus;
}
result += $"Bonus was {(totalBonus / 100).ToString("c", localFormat)}\n";
result += $"You got {totalExperience} experiences\n";
return result;
}
After compilation and testing, this refactoring works. Then use Inline Variable.
public string Statement(HuntingMission mission, Dictionary<string, Monster> monsters)
{
Func<HuntingTarget, Monster> monsterFor = delegate(HuntingTarget missionTarget)
{
return monsters\[missionTarget.MonsterId\];
};
Func<HuntingTarget, Monster, int> amountFor = delegate (HuntingTarget missionTarget, Monster monster)
{
int result = 0;
switch (monster.Type)
{
case "giant":
result = 60000;
if (missionTarget.Level > 25)
{
result += 500 * (missionTarget.Level - 25);
}
break;
case "dragon":
result = 80000;
if (missionTarget.Level > 35)
{
result += 2500 + 900 * (missionTarget.Level - 35);
}
result += 200 * missionTarget.Level;
break;
default:
throw new Exception($"Unknown type: {monster.Type}");
}
return result;
};
int totalExperience = 0;
int totalBonus = 0;
string result = $"Statement for {mission.Name}\n";
CultureInfo enCulture = new CultureInfo("en-US");
Thread.CurrentThread.CurrentCulture = enCulture;
NumberFormatInfo localFormat = (NumberFormatInfo)NumberFormatInfo.CurrentInfo.Clone();
foreach (var missionTarget in mission.Targets)
{
int thisBonus = amountFor(missionTarget, monsterFor(missionTarget));
// add experience
totalExperience += Math.Max(missionTarget.Level - 40, 0);
// add extra experience for every dragon monster by level dividing by 3
if (monsterFor(missionTarget).Type == "dragon")
{
totalExperience += missionTarget.Level / 3;
}
result +=
$"{monsterFor(missionTarget).Name}: {(thisBonus / 100).ToString("c", localFormat)} ({missionTarget.Level} levels)\n";
totalBonus += thisBonus;
}
result += $"Bonus was {(totalBonus / 100).ToString("c", localFormat)}\n";
result += $"You got {totalExperience} experiences\n";
return result;
}
After compilation and testing, this refactoring works. Then use Change Function Declaration to amountFor to remove the monster variable_._ There are 2 steps, in first step I change it to use monsterFor:
Func<HuntingTarget, Monster, int> amountFor = delegate (HuntingTarget missionTarget, Monster monster)
{
int result = 0;
switch (monsterFor(missionTarget).Type)
{
case "giant":
result = 60000;
if (missionTarget.Level > 25)
{
result += 500 * (missionTarget.Level - 25);
}
break;
case "dragon":
result = 80000;
if (missionTarget.Level > 35)
{
result += 2500 + 900 * (missionTarget.Level - 35);
}
result += 200 * missionTarget.Level;
break;
default:
throw new Exception($"Unknown type: {monsterFor(missionTarget).Type}");
}
return result;
};
After compilation and testing, this refactoring works. In second step I remove the monster parameter of amountFor function:
public string Statement(HuntingMission mission, Dictionary<string, Monster> monsters)
{
Func<HuntingTarget, Monster> monsterFor = delegate(HuntingTarget missionTarget)
{
return monsters\[missionTarget.MonsterId\];
};
Func<HuntingTarget, Monster, int> amountFor = delegate (HuntingTarget missionTarget, Monster monster)
{
int result = 0;
switch (monsterFor(missionTarget).Type)
{
case "giant":
result = 60000;
if (missionTarget.Level > 25)
{
result += 500 * (missionTarget.Level - 25);
}
break;
case "dragon":
result = 80000;
if (missionTarget.Level > 35)
{
result += 2500 + 900 * (missionTarget.Level - 35);
}
result += 200 * missionTarget.Level;
break;
default:
throw new Exception($"Unknown type: {monsterFor(missionTarget).Type}");
}
return result;
};
int totalExperience = 0;
int totalBonus = 0;
string result = $"Statement for {mission.Name}\n";
CultureInfo enCulture = new CultureInfo("en-US");
Thread.CurrentThread.CurrentCulture = enCulture;
NumberFormatInfo localFormat = (NumberFormatInfo)NumberFormatInfo.CurrentInfo.Clone();
foreach (var missionTarget in mission.Targets)
{
int thisBonus = amountFor(missionTarget, monsterFor(missionTarget));
// add experience
totalExperience += Math.Max(missionTarget.Level - 40, 0);
// add extra experience for every dragon monster by level dividing by 3
if (monsterFor(missionTarget).Type == "dragon")
{
totalExperience += missionTarget.Level / 3;
}
result +=
$"{monsterFor(missionTarget).Name}: {(thisBonus / 100).ToString("c", localFormat)} ({missionTarget.Level} levels)\n";
totalBonus += thisBonus;
}
result += $"Bonus was {(totalBonus / 100).ToString("c", localFormat)}\n";
result += $"You got {totalExperience} experiences\n";
return result;
}
After compilation and testing, this refactoring works. I feel concerned that monsterFor is called 4 times and is more than the first version (called 1 time). It seems a bit performance problem but it's not a serious situation. Martin said he will discuss in later chapters.
Let's use Inline Variable again_._ thisBonus variable is assigned and not modified so use amountFor to replace it:
public string Statement(HuntingMission mission, Dictionary<string, Monster> monsters)
{
Func<HuntingTarget, Monster> monsterFor = delegate(HuntingTarget missionTarget)
{
return monsters\[missionTarget.MonsterId\];
};
Func<HuntingTarget, int> amountFor = delegate (HuntingTarget missionTarget)
{
int result = 0;
switch (monsterFor(missionTarget).Type)
{
case "giant":
result = 60000;
if (missionTarget.Level > 25)
{
result += 500 * (missionTarget.Level - 25);
}
break;
case "dragon":
result = 80000;
if (missionTarget.Level > 35)
{
result += 2500 + 900 * (missionTarget.Level - 35);
}
result += 200 * missionTarget.Level;
break;
default:
throw new Exception($"Unknown type: {monsterFor(missionTarget).Type}");
}
return result;
};
int totalExperience = 0;
int totalBonus = 0;
string result = $"Statement for {mission.Name}\n";
CultureInfo enCulture = new CultureInfo("en-US");
Thread.CurrentThread.CurrentCulture = enCulture;
NumberFormatInfo localFormat = (NumberFormatInfo)NumberFormatInfo.CurrentInfo.Clone();
foreach (var missionTarget in mission.Targets)
{
// add experience
totalExperience += Math.Max(missionTarget.Level - 40, 0);
// add extra experience for every dragon monster by level dividing by 3
if (monsterFor(missionTarget).Type == "dragon")
{
totalExperience += missionTarget.Level / 3;
}
result +=
$"{monsterFor(missionTarget).Name}: {(amountFor(missionTarget) / 100).ToString("c", localFormat)} ({missionTarget.Level} levels)\n";
totalBonus += amountFor(missionTarget);
}
result += $"Bonus was {(totalBonus / 100).ToString("c", localFormat)}\n";
result += $"You got {totalExperience} experiences\n";
return result;
}
totalExperience is updated in the for loop. Let's extract the a function that calculates the new experience value and returns it to totalExperience:
public string Statement(HuntingMission mission, Dictionary<string, Monster> monsters)
{
Func<HuntingTarget, Monster> monsterFor = delegate (HuntingTarget missionTarget)
{
return monsters\[missionTarget.MonsterId\];
};
Func<HuntingTarget, int> experienceFor = delegate (HuntingTarget missionTarget)
{
int experience = 0;
experience += Math.Max(missionTarget.Level - 40, 0);
// add extra experience for every dragon monster by level dividing by 3
if (monsterFor(missionTarget).Type == "dragon")
{
experience += missionTarget.Level / 3;
}
return experience;
};
Func<HuntingTarget, int> amountFor = delegate (HuntingTarget missionTarget)
{
int result = 0;
switch (monsterFor(missionTarget).Type)
{
case "giant":
result = 60000;
if (missionTarget.Level > 25)
{
result += 500 * (missionTarget.Level - 25);
}
break;
case "dragon":
result = 80000;
if (missionTarget.Level > 35)
{
result += 2500 + 900 * (missionTarget.Level - 35);
}
result += 200 * missionTarget.Level;
break;
default:
throw new Exception($"Unknown type: {monsterFor(missionTarget).Type}");
}
return result;
};
int totalExperience = 0;
int totalBonus = 0;
string result = $"Statement for {mission.Name}\n";
CultureInfo enCulture = new CultureInfo("en-US");
Thread.CurrentThread.CurrentCulture = enCulture;
NumberFormatInfo localFormat = (NumberFormatInfo)NumberFormatInfo.CurrentInfo.Clone();
foreach (var missionTarget in mission.Targets)
{
// add experience
totalExperience += experienceFor(missionTarget);
result +=
$"{monsterFor(missionTarget).Name}: {(amountFor(missionTarget) / 100).ToString("c", localFormat)} ({missionTarget.Level} levels)\n";
totalBonus += amountFor(missionTarget);
}
result += $"Bonus was {(totalBonus / 100).ToString("c", localFormat)}\n";
result += $"You got {totalExperience} experiences\n";
return result;
}
Func<HuntingTarget, int> experienceFor = delegate (HuntingTarget missionTarget)
{
int result = 0;
result += Math.Max(missionTarget.Level - 40, 0);
// add extra experience for every dragon monster by level dividing by 3
if (monsterFor(missionTarget).Type == "dragon")
{
result += missionTarget.Level / 3;
}
return result;
};
Temporary variables are underlying problems. It's suggested to transfer them into functions. Let's create a declared function to replace localFormat:
public string Statement(HuntingMission mission, Dictionary<string, Monster> monsters)
{
Func<HuntingTarget, Monster> monsterFor = delegate (HuntingTarget missionTarget)
{
return monsters\[missionTarget.MonsterId\];
};
Func<HuntingTarget, int> experienceFor = delegate (HuntingTarget missionTarget)
{
int result = 0;
result += Math.Max(missionTarget.Level - 40, 0);
// add extra experience for every dragon monster by level dividing by 3
if (monsterFor(missionTarget).Type == "dragon")
{
result += missionTarget.Level / 3;
}
return result;
};
Func<int, string> format = delegate(int aNumber)
{
CultureInfo enCulture = new CultureInfo("en-US");
Thread.CurrentThread.CurrentCulture = enCulture;
NumberFormatInfo localFormat = (NumberFormatInfo)NumberFormatInfo.CurrentInfo.Clone();
return aNumber.ToString("c", localFormat);
};
Func<HuntingTarget, int> amountFor = delegate (HuntingTarget missionTarget)
{
int result = 0;
switch (monsterFor(missionTarget).Type)
{
case "giant":
result = 60000;
if (missionTarget.Level > 25)
{
result += 500 * (missionTarget.Level - 25);
}
break;
case "dragon":
result = 80000;
if (missionTarget.Level > 35)
{
result += 2500 + 900 * (missionTarget.Level - 35);
}
result += 200 * missionTarget.Level;
break;
default:
throw new Exception($"Unknown type: {monsterFor(missionTarget).Type}");
}
return result;
};
int totalExperience = 0;
int totalBonus = 0;
string result = $"Statement for {mission.Name}\n";
foreach (var missionTarget in mission.Targets)
{
// add experience
totalExperience += experienceFor(missionTarget);
result +=
$"{monsterFor(missionTarget).Name}: {format(amountFor(missionTarget) / 100)} ({missionTarget.Level} levels)\n";
totalBonus += amountFor(missionTarget);
}
result += $"Bonus was {format(totalBonus / 100)}\n";
result += $"You got {totalExperience} experiences\n";
return result;
}
public string Statement(HuntingMission mission, Dictionary<string, Monster> monsters)
{
Func<HuntingTarget, Monster> monsterFor = delegate (HuntingTarget missionTarget)
{
return monsters\[missionTarget.MonsterId\];
};
Func<HuntingTarget, int> experienceFor = delegate (HuntingTarget missionTarget)
{
int result = 0;
result += Math.Max(missionTarget.Level - 40, 0);
// add extra experience for every dragon monster by level dividing by 3
if (monsterFor(missionTarget).Type == "dragon")
{
result += missionTarget.Level / 3;
}
return result;
};
Func<int, string> usd = delegate(int aNumber)
{
CultureInfo enCulture = new CultureInfo("en-US");
Thread.CurrentThread.CurrentCulture = enCulture;
NumberFormatInfo localFormat = (NumberFormatInfo)NumberFormatInfo.CurrentInfo.Clone();
return (aNumber / 100).ToString("c", localFormat);
};
Func<HuntingTarget, int> amountFor = delegate (HuntingTarget missionTarget)
{
int result = 0;
switch (monsterFor(missionTarget).Type)
{
case "giant":
result = 60000;
if (missionTarget.Level > 25)
{
result += 500 * (missionTarget.Level - 25);
}
break;
case "dragon":
result = 80000;
if (missionTarget.Level > 35)
{
result += 2500 + 900 * (missionTarget.Level - 35);
}
result += 200 * missionTarget.Level;
break;
default:
throw new Exception($"Unknown type: {monsterFor(missionTarget).Type}");
}
return result;
};
int totalExperience = 0;
int totalBonus = 0;
string result = $"Statement for {mission.Name}\n";
foreach (var missionTarget in mission.Targets)
{
// add experience
totalExperience += experienceFor(missionTarget);
result +=
$"{monsterFor(missionTarget).Name}: {usd(amountFor(missionTarget))} ({missionTarget.Level} levels)\n";
totalBonus += amountFor(missionTarget);
}
result += $"Bonus was {usd(totalBonus)}\n";
result += $"You got {totalExperience} experiences\n";
return result;
}
Good function naming means that you can read the functionality without reading its underlying inner struct. Naming talks you what they do.
This totalExperience variable is accumulated in a for loop. Let's use Split Loop to separate it:
public string Statement(HuntingMission mission, Dictionary<string, Monster> monsters)
{
int totalExperience = 0;
int totalBonus = 0;
string result = $"Statement for {mission.Name}\n";
foreach (var missionTarget in mission.Targets)
{
result +=
$"{monsterFor(missionTarget).Name}: {usd(amountFor(missionTarget))} ({missionTarget.Level} levels)\n";
totalBonus += amountFor(missionTarget);
}
foreach (var missionTarget in mission.Targets)
{
// add experience
totalExperience += experienceFor(missionTarget);
}
result += $"Bonus was {usd(totalBonus)}\n";
result += $"You got {totalExperience} experiences\n";
return result;
}
Then use Slide Statements to put the declaration of the variable close to the loop:
public string Statement(HuntingMission mission, Dictionary<string, Monster> monsters)
{
int totalBonus = 0;
string result = $"Statement for {mission.Name}\n";
foreach (var missionTarget in mission.Targets)
{
result +=
$"{monsterFor(missionTarget).Name}: {usd(amountFor(missionTarget))} ({missionTarget.Level} levels)\n";
totalBonus += amountFor(missionTarget);
}
int totalExperience = 0;
foreach (var missionTarget in mission.Targets)
{
// add experience
totalExperience += experienceFor(missionTarget);
}
result += $"Bonus was {usd(totalBonus)}\n";
result += $"You got {totalExperience} experiences\n";
return result;
}
Then use Extract Function to the totalExperience variable:
public string Statement(HuntingMission mission, Dictionary<string, Monster> monsters)
{
Func<HuntingTarget, Monster> monsterFor = delegate (HuntingTarget missionTarget)
{
return monsters\[missionTarget.MonsterId\];
};
Func<HuntingTarget, int> experienceFor = delegate (HuntingTarget missionTarget)
{
int result = 0;
result += Math.Max(missionTarget.Level - 40, 0);
// add extra experience for every dragon monster by level dividing by 3
if (monsterFor(missionTarget).Type == "dragon")
{
result += missionTarget.Level / 3;
}
return result;
};
Func<int, string> usd = delegate(int aNumber)
{
CultureInfo enCulture = new CultureInfo("en-US");
Thread.CurrentThread.CurrentCulture = enCulture;
NumberFormatInfo localFormat = (NumberFormatInfo)NumberFormatInfo.CurrentInfo.Clone();
return (aNumber / 100).ToString("c", localFormat);
};
Func<int> totalExperiences = delegate()
{
int totalExperience = 0;
foreach (var missionTarget in mission.Targets)
{
// add experience
totalExperience += experienceFor(missionTarget);
}
return totalExperience;
};
Func<HuntingTarget, int> amountFor = delegate (HuntingTarget missionTarget)
{
int result = 0;
switch (monsterFor(missionTarget).Type)
{
case "giant":
result = 60000;
if (missionTarget.Level > 25)
{
result += 500 * (missionTarget.Level - 25);
}
break;
case "dragon":
result = 80000;
if (missionTarget.Level > 35)
{
result += 2500 + 900 * (missionTarget.Level - 35);
}
result += 200 * missionTarget.Level;
break;
default:
throw new Exception($"Unknown type: {monsterFor(missionTarget).Type}");
}
return result;
};
int totalBonus = 0;
string result = $"Statement for {mission.Name}\n";
foreach (var missionTarget in mission.Targets)
{
result +=
$"{monsterFor(missionTarget).Name}: {usd(amountFor(missionTarget))} ({missionTarget.Level} levels)\n";
totalBonus += amountFor(missionTarget);
}
int totalExperience = totalExperiences();
result += $"Bonus was {usd(totalBonus)}\n";
result += $"You got {totalExperience} experiences\n";
return result;
}
Next is Inline Variable to the totalExperience variable:
public string Statement(HuntingMission mission, Dictionary<string, Monster> monsters)
{
int totalBonus = 0;
string result = $"Statement for {mission.Name}\n";
foreach (var missionTarget in mission.Targets)
{
result +=
$"{monsterFor(missionTarget).Name}: {usd(amountFor(missionTarget))} ({missionTarget.Level} levels)\n";
totalBonus += amountFor(missionTarget);
}
result += $"Bonus was {usd(totalBonus)}\n";
result += $"You got {totalExperiences()} experiences\n";
return result;
}
For totalBonus variable, I refactor it with the same refactoring skill done with totalExperience variable. First I use Extract Function to the totalBonus variable:
public string Statement(HuntingMission mission, Dictionary<string, Monster> monsters)
{
Func<HuntingTarget, Monster> monsterFor = delegate (HuntingTarget missionTarget)
{
return monsters\[missionTarget.MonsterId\];
};
Func<HuntingTarget, int> experienceFor = delegate (HuntingTarget missionTarget)
{
int result = 0;
result += Math.Max(missionTarget.Level - 40, 0);
// add extra experience for every dragon monster by level dividing by 3
if (monsterFor(missionTarget).Type == "dragon")
{
result += missionTarget.Level / 3;
}
return result;
};
Func<int, string> usd = delegate(int aNumber)
{
CultureInfo enCulture = new CultureInfo("en-US");
Thread.CurrentThread.CurrentCulture = enCulture;
NumberFormatInfo localFormat = (NumberFormatInfo)NumberFormatInfo.CurrentInfo.Clone();
return (aNumber / 100).ToString("c", localFormat);
};
Func<int> totalExperiences = delegate()
{
int totalExperience = 0;
foreach (var missionTarget in mission.Targets)
{
// add experience
totalExperience += experienceFor(missionTarget);
}
return totalExperience;
};
Func<HuntingTarget, int> amountFor = delegate (HuntingTarget missionTarget)
{
int result = 0;
switch (monsterFor(missionTarget).Type)
{
case "giant":
result = 60000;
if (missionTarget.Level > 25)
{
result += 500 * (missionTarget.Level - 25);
}
break;
case "dragon":
result = 80000;
if (missionTarget.Level > 35)
{
result += 2500 + 900 * (missionTarget.Level - 35);
}
result += 200 * missionTarget.Level;
break;
default:
throw new Exception($"Unknown type: {monsterFor(missionTarget).Type}");
}
return result;
};
Func<int> totalBonuses = delegate ()
{
int totalBonus = 0;
foreach (var missionTarget in mission.Targets)
{
totalBonus += amountFor(missionTarget);
}
return totalBonus;
};
string result = $"Statement for {mission.Name}\n";
foreach (var missionTarget in mission.Targets)
{
result +=
$"{monsterFor(missionTarget).Name}: {usd(amountFor(missionTarget))} ({missionTarget.Level} levels)\n";
}
int totalBonus = totalBonuses();
result += $"Bonus was {usd(totalBonus)}\n";
result += $"You got {totalExperiences()} experiences\n";
return result;
}
Next is Inline Variable to the totalBonus variable:
public string Statement(HuntingMission mission, Dictionary<string, Monster> monsters)
{
string result = $"Statement for {mission.Name}\n";
foreach (var missionTarget in mission.Targets)
{
result +=
$"{monsterFor(missionTarget).Name}: {usd(amountFor(missionTarget))} ({missionTarget.Level} levels)\n";
}
result += $"Bonus was {usd(totalBonuses())}\n";
result += $"You got {totalExperiences()} experiences\n";
return result;
}
Let's rename the calculated variables to result:
Func<int> totalExperiences = delegate()
{
int result = 0;
foreach (var missionTarget in mission.Targets)
{
// add experience
result += experienceFor(missionTarget);
}
return result;
};
Func<int> totalBonuses = delegate ()
{
int result = 0;
foreach (var missionTarget in mission.Targets)
{
result += amountFor(missionTarget);
}
return result;
};
The above Statement function can return the bonus/experience by the monster list and the hunting mission. But the bonus/experience format is a kind of version. If the format is extended into HTML version, how should I refactor it?
Let's use Split Phase to split the function into the logic of two phases: calculating the data structure of bonus/experience and rendering the format(pure string or HTML) by the data structure.
Extract Function is applied to the second phase that is rendering the format. The whole Statement's content is transferred into another function RenderPlainText:
public string Statement(HuntingMission mission, Dictionary<string, Monster> monsters)
{
return RenderPlainText(mission, monsters);
}
private string RenderPlainText(HuntingMission mission, Dictionary<string, Monster> monsters)
{
Func<HuntingTarget, Monster> monsterFor = delegate (HuntingTarget missionTarget)
{
...
};
Func<HuntingTarget, int> experienceFor = delegate (HuntingTarget missionTarget)
{
...
};
Func<int, string> usd = delegate (int aNumber)
{
...
};
Func<int> totalExperiences = delegate ()
{
...
};
Func<HuntingTarget, int> amountFor = delegate (HuntingTarget missionTarget)
{
...
};
Func<int> totalBonuses = delegate ()
{
...
};
string result = $"Statement for {mission.Name}\n";
foreach (var missionTarget in mission.Targets)
{
result +=
$"{monsterFor(missionTarget).Name}: {usd(amountFor(missionTarget))} ({missionTarget.Level} levels)\n";
}
result += $"Bonus was {usd(totalBonuses())}\n";
result += $"You got {totalExperiences()} experiences\n";
return result;
}
public class MissionManager
{
public string Statement(HuntingMission mission, Dictionary<string, Monster> monsters)
{
StatementData statementData = new StatementData();
return RenderPlainText(statementData, mission, monsters);
}
private string RenderPlainText(StatementData data, HuntingMission mission, Dictionary<string, Monster> monsters)
{
Func<HuntingTarget, Monster> monsterFor = delegate (HuntingTarget missionTarget)
{
...
};
Func<HuntingTarget, int> experienceFor = delegate (HuntingTarget missionTarget)
{
...
};
Func<int, string> usd = delegate (int aNumber)
{
...
};
Func<int> totalExperiences = delegate ()
{
...
};
Func<HuntingTarget, int> amountFor = delegate (HuntingTarget missionTarget)
{
...
};
Func<int> totalBonuses = delegate ()
{
...
};
string result = $"Statement for {mission.Name}\n";
foreach (var missionTarget in mission.Targets)
{
result +=
$"{monsterFor(missionTarget).Name}: {usd(amountFor(missionTarget))} ({missionTarget.Level} levels)\n";
}
result += $"Bonus was {usd(totalBonuses())}\n";
result += $"You got {totalExperiences()} experiences\n";
return result;
}
}
internal class StatementData
{
}
Current SatatementData class has no any property. After compilation and testing, this refactoring works.
Make RenderPlainText to use required parameter in StatementData instance. First step move mission name to this statementData:
public string Statement(HuntingMission mission, Dictionary<string, Monster> monsters)
{
StatementData statementData = new StatementData();
statementData.MissionName = mission.Name;
return RenderPlainText(statementData, mission, monsters);
}
private string RenderPlainText(StatementData data, HuntingMission mission, Dictionary<string, Monster> monsters)
{
//all delegate functions are hidden
string result = $"Statement for {data.MissionName}\n";
foreach (var missionTarget in mission.Targets)
{
result +=
$"{monsterFor(missionTarget).Name}: {usd(amountFor(missionTarget))} ({missionTarget.Level} levels)\n";
}
result += $"Bonus was {usd(totalBonuses())}\n";
result += $"You got {totalExperiences()} experiences\n";
return result;
}
internal class StatementData
{
public string MissionName { get; set; }
}
After compilation and testing, this refactoring works. Then also move Targets to statementData and delete the mission variable:
public class MissionManager
{
public string Statement(HuntingMission mission, Dictionary<string, Monster> monsters)
{
StatementData statementData = new StatementData();
statementData.MissionName = mission.Name;
statementData.Targets = mission.Targets;
return RenderPlainText(statementData, mission, monsters);
}
private string RenderPlainText(StatementData data, Dictionary<string, Monster> monsters)
{
Func<HuntingTarget, Monster> monsterFor = delegate (HuntingTarget missionTarget)
{
...
};
Func<HuntingTarget, int> experienceFor = delegate (HuntingTarget missionTarget)
{
...
};
Func<int, string> usd = delegate (int aNumber)
{
...
};
Func<int> totalExperiences = delegate ()
{
int result = 0;
foreach (var missionTarget in data.Targets)
{
// add experience
result += experienceFor(missionTarget);
}
return result;
};
Func<HuntingTarget, int> amountFor = delegate (HuntingTarget missionTarget)
{
...
};
Func<int> totalBonuses = delegate ()
{
int result = 0;
foreach (var missionTarget in data.Targets)
{
result += amountFor(missionTarget);
}
return result;
};
string result = $"Statement for {data.MissionName}\n";
foreach (var missionTarget in data.Targets)
{
result +=
$"{monsterFor(missionTarget).Name}: {usd(amountFor(missionTarget))} ({missionTarget.Level} levels)\n";
}
result += $"Bonus was {usd(totalBonuses())}\n";
result += $"You got {totalExperiences()} experiences\n";
return result;
}
}
internal class StatementData
{
public string MissionName { get; set; }
public List<HuntingTarget> Targets { get; set; }
}
After compilation and testing, this refactoring works. The StatementData's Targets variable then is set as an immutable data by enrichTarget delegate function:
public string Statement(HuntingMission mission, Dictionary<string, Monster> monsters)
{
Func<HuntingTarget, HuntingTarget> enrichTarget = delegate(HuntingTarget target)
{
HuntingTarget result = target;
return result;
};
StatementData statementData = new StatementData();
statementData.MissionName = mission.Name;
statementData.Targets = mission.Targets.Select(enrichTarget).ToList();
return RenderPlainText(statementData, monsters);
}
Use Move Function skill to Statement and monsterFor functions, then all original references to monsterFor are replaced with data instance. Before I move them, I create another class CalculatedTarget. Its properties are the same as the class HuntingTarget and it has extra property Monster. CalculatedTarget's instances are replaced with HuntingMission's Targets. Then RenderPlainText's referenced delegate functions are updated to use CalculatedTarget:
public class MissionManager
{
public string Statement(HuntingMission mission, Dictionary<string, Monster> monsters)
{
Func<CalculatedTarget, Monster> monsterFor = delegate (CalculatedTarget missionTarget)
{
return monsters\[missionTarget.MonsterId\];
};
Func<HuntingTarget, CalculatedTarget> enrichTarget = delegate(HuntingTarget target)
{
CalculatedTarget result = new CalculatedTarget(target.MonsterId, target.Level);
result.Monster = monsterFor(result);
return result;
};
StatementData statementData = new StatementData();
statementData.MissionName = mission.Name;
statementData.Targets = mission.Targets.Select(enrichTarget).ToList();
return RenderPlainText(statementData, monsters);
}
private string RenderPlainText(StatementData data, Dictionary<string, Monster> monsters)
{
Func<CalculatedTarget, int> experienceFor = delegate (CalculatedTarget missionTarget)
{
int result = 0;
result += Math.Max(missionTarget.Level - 40, 0);
// add extra experience for every dragon monster by level dividing by 3
if (missionTarget.Monster.Type == "dragon")
{
result += missionTarget.Level / 3;
}
return result;
};
Func<int, string> usd = delegate (int aNumber)
{
...
};
Func<int> totalExperiences = delegate ()
{
...
};
Func<CalculatedTarget, int> amountFor = delegate (CalculatedTarget missionTarget)
{
int result = 0;
switch (missionTarget.Monster.Type)
{
case "giant":
result = 60000;
if (missionTarget.Level > 25)
{
result += 500 * (missionTarget.Level - 25);
}
break;
case "dragon":
result = 80000;
if (missionTarget.Level > 35)
{
result += 2500 + 900 * (missionTarget.Level - 35);
}
result += 200 * missionTarget.Level;
break;
default:
throw new Exception($"Unknown type: {missionTarget.Monster.Type}");
}
return result;
};
Func<int> totalBonuses = delegate ()
{
...
};
string result = $"Statement for {data.MissionName}\n";
foreach (var missionTarget in data.Targets)
{
result +=
$"{missionTarget.Monster.Name}: {usd(amountFor(missionTarget))} ({missionTarget.Level} levels)\n";
}
result += $"Bonus was {usd(totalBonuses())}\n";
result += $"You got {totalExperiences()} experiences\n";
return result;
}
}
internal class StatementData
{
public string MissionName { get; set; }
public List<CalculatedTarget> Targets { get; set; }
}
internal class CalculatedTarget
{
public string MonsterId { get; }
public int Level { get; }
public Monster Monster { get; set; }
public CalculatedTarget(string monsterId, int level)
{
MonsterId = monsterId;
Level = level;
}
}
After compilation and testing, this refactoring works. Then use the same Move Function skill to Statement and amountFor functions:
public class MissionManager
{
public string Statement(HuntingMission mission, Dictionary<string, Monster> monsters)
{
Func<CalculatedTarget, Monster> monsterFor = delegate (CalculatedTarget missionTarget)
{
...
};
Func<CalculatedTarget, int> amountFor = delegate (CalculatedTarget missionTarget)
{
int result = 0;
switch (missionTarget.Monster.Type)
{
case "giant":
result = 60000;
if (missionTarget.Level > 25)
{
result += 500 * (missionTarget.Level - 25);
}
break;
case "dragon":
result = 80000;
if (missionTarget.Level > 35)
{
result += 2500 + 900 * (missionTarget.Level - 35);
}
result += 200 * missionTarget.Level;
break;
default:
throw new Exception($"Unknown type: {missionTarget.Monster.Type}");
}
return result;
};
Func<HuntingTarget, CalculatedTarget> enrichTarget = delegate(HuntingTarget target)
{
CalculatedTarget result = new CalculatedTarget(target.MonsterId, target.Level);
result.Monster = monsterFor(result);
result.Amount = amountFor(result);
return result;
};
StatementData statementData = new StatementData();
statementData.MissionName = mission.Name;
statementData.Targets = mission.Targets.Select(enrichTarget).ToList();
return RenderPlainText(statementData, monsters);
}
private string RenderPlainText(StatementData data, Dictionary<string, Monster> monsters)
{
Func<CalculatedTarget, int> experienceFor = delegate (CalculatedTarget missionTarget)
{
...
};
Func<int, string> usd = delegate (int aNumber)
{
...
};
Func<int> totalExperiences = delegate ()
{
...
};
Func<int> totalBonuses = delegate ()
{
int result = 0;
foreach (var missionTarget in data.Targets)
{
result += missionTarget.Amount;
}
return result;
};
string result = $"Statement for {data.MissionName}\n";
foreach (var missionTarget in data.Targets)
{
result +=
$"{missionTarget.Monster.Name}: {usd(missionTarget.Amount)} ({missionTarget.Level} levels)\n";
}
result += $"Bonus was {usd(totalBonuses())}\n";
result += $"You got {totalExperiences()} experiences\n";
return result;
}
}
internal class CalculatedTarget
{
public string MonsterId { get; }
public int Level { get; }
public Monster Monster { get; set; }
public int Amount { get; set; }
public CalculatedTarget(string monsterId, int level)
{
MonsterId = monsterId;
Level = level;
}
}
After compilation and testing, this refactoring works. Then use the same Move Function skill to Statement and experienceFor functions:
public class MissionManager
{
public string Statement(HuntingMission mission, Dictionary<string, Monster> monsters)
{
Func<CalculatedTarget, Monster> monsterFor = delegate (CalculatedTarget missionTarget)
{
...
};
Func<CalculatedTarget, int> amountFor = delegate (CalculatedTarget missionTarget)
{
...
};
Func<CalculatedTarget, int> experienceFor = delegate (CalculatedTarget missionTarget)
{
int result = 0;
result += Math.Max(missionTarget.Level - 40, 0);
// add extra experience for every dragon monster by level dividing by 3
if (missionTarget.Monster.Type == "dragon")
{
result += missionTarget.Level / 3;
}
return result;
};
Func<HuntingTarget, CalculatedTarget> enrichTarget = delegate(HuntingTarget target)
{
CalculatedTarget result = new CalculatedTarget(target.MonsterId, target.Level);
result.Monster = monsterFor(result);
result.Amount = amountFor(result);
result.Experience = experienceFor(result);
return result;
};
StatementData statementData = new StatementData();
statementData.MissionName = mission.Name;
statementData.Targets = mission.Targets.Select(enrichTarget).ToList();
return RenderPlainText(statementData, monsters);
}
private string RenderPlainText(StatementData data, Dictionary<string, Monster> monsters)
{
Func<int, string> usd = delegate (int aNumber)
{
...
};
Func<int> totalExperiences = delegate ()
{
int result = 0;
foreach (var missionTarget in data.Targets)
{
// add experience
result += missionTarget.Experience;
}
return result;
};
Func<int> totalBonuses = delegate ()
{
...
};
string result = $"Statement for {data.MissionName}\n";
foreach (var missionTarget in data.Targets)
{
result +=
$"{missionTarget.Monster.Name}: {usd(missionTarget.Amount)} ({missionTarget.Level} levels)\n";
}
result += $"Bonus was {usd(totalBonuses())}\n";
result += $"You got {totalExperiences()} experiences\n";
return result;
}
}
internal class CalculatedTarget
{
public string MonsterId { get; }
public int Level { get; }
public Monster Monster { get; set; }
public int Amount { get; set; }
public int Experience { get; set; }
public CalculatedTarget(string monsterId, int level)
{
MonsterId = monsterId;
Level = level;
}
}
After compilation and testing, this refactoring works. Finally, use Move Function skill to Statement, totalExperiences and totalBonuses functions:
public class MissionManager
{
public string Statement(HuntingMission mission, Dictionary<string, Monster> monsters)
{
Func<CalculatedTarget, Monster> monsterFor = delegate (CalculatedTarget missionTarget)
{
...
};
Func<CalculatedTarget, int> amountFor = delegate (CalculatedTarget missionTarget)
{
...
};
Func<CalculatedTarget, int> experienceFor = delegate (CalculatedTarget missionTarget)
{
...
};
Func<HuntingTarget, CalculatedTarget> enrichTarget = delegate(HuntingTarget target)
{
...
};
Func<StatementData, int> totalExperiences = delegate (StatementData data)
{
int result = 0;
foreach (var missionTarget in data.Targets)
{
// add experience
result += missionTarget.Experience;
}
return result;
};
Func<StatementData, int> totalBonuses = delegate (StatementData data)
{
int result = 0;
foreach (var missionTarget in data.Targets)
{
result += missionTarget.Amount;
}
return result;
};
StatementData statementData = new StatementData();
statementData.MissionName = mission.Name;
statementData.Targets = mission.Targets.Select(enrichTarget).ToList();
statementData.TotalBonuses = totalBonuses(statementData);
statementData.TotalExperiences = totalExperiences(statementData);
return RenderPlainText(statementData, monsters);
}
private string RenderPlainText(StatementData data, Dictionary<string, Monster> monsters)
{
Func<int, string> usd = delegate (int aNumber)
{
...
};
string result = $"Statement for {data.MissionName}\n";
foreach (var missionTarget in data.Targets)
{
result +=
$"{missionTarget.Monster.Name}: {usd(missionTarget.Amount)} ({missionTarget.Level} levels)\n";
}
result += $"Bonus was {usd(data.TotalBonuses)}\n";
result += $"You got {data.TotalExperiences} experiences\n";
return result;
}
}
internal class StatementData
{
public string MissionName { get; set; }
public List<CalculatedTarget> Targets { get; set; }
public int TotalBonuses { get; set; }
public int TotalExperiences { get; set; }
}
After compilation and testing, this refactoring works. Some functions are refactored with Replace Loop with Pipeline:
Func<StatementData, int> totalExperiences = delegate (StatementData data)
{
return data.Targets.Sum(x => x.Experience);
};
Func<StatementData, int> totalBonuses = delegate (StatementData data)
{
return data.Targets.Sum(x => x.Amount);
};
After compilation and testing, this refactoring works. Extract the code into a independent function in the first phase (Statement Function) and remove the monsters argument of RenderPlainText function:
public string Statement(HuntingMission mission, Dictionary<string, Monster> monsters)
{
return RenderPlainText(CreateStatementData(mission, monsters));
}
private StatementData CreateStatementData(HuntingMission mission, Dictionary<string, Monster> monsters)
{
Func<CalculatedTarget, Monster> monsterFor = delegate (CalculatedTarget missionTarget)
{
...
};
Func<CalculatedTarget, int> amountFor = delegate (CalculatedTarget missionTarget)
{
...
};
Func<CalculatedTarget, int> experienceFor = delegate (CalculatedTarget missionTarget)
{
...
};
Func<HuntingTarget, CalculatedTarget> enrichTarget = delegate (HuntingTarget target)
{
...
};
Func<StatementData, int> totalExperiences = delegate (StatementData data)
{
...
};
Func<StatementData, int> totalBonuses = delegate (StatementData data)
{
...
};
StatementData statementData = new StatementData();
statementData.MissionName = mission.Name;
statementData.Targets = mission.Targets.Select(enrichTarget).ToList();
statementData.TotalBonuses = totalBonuses(statementData);
statementData.TotalExperiences = totalExperiences(statementData);
return statementData;
}
private string RenderPlainText(StatementData data, Dictionary<string, Monster> monsters)
{
...
}
After compilation and testing, this refactoring works. Then extract this CreateStatementData function into another class StatementDataManager:
public class MissionManager
{
public string Statement(HuntingMission mission, Dictionary<string, Monster> monsters)
{
return RenderPlainText(new StatementDataManager().CreateStatementData(mission, monsters));
}
}
internal class StatementDataManager
{
public StatementData CreateStatementData(HuntingMission mission, Dictionary<string, Monster> monsters)
{
Func<CalculatedTarget, Monster> monsterFor = delegate (CalculatedTarget missionTarget)
{
...
};
Func<CalculatedTarget, int> amountFor = delegate (CalculatedTarget missionTarget)
{
...
};
Func<CalculatedTarget, int> experienceFor = delegate (CalculatedTarget missionTarget)
{
...
};
Func<HuntingTarget, CalculatedTarget> enrichTarget = delegate (HuntingTarget target)
{
...
};
Func<StatementData, int> totalExperiences = delegate (StatementData data)
{
...
};
Func<StatementData, int> totalBonuses = delegate (StatementData data)
{
...
};
StatementData statementData = new StatementData();
statementData.MissionName = mission.Name;
statementData.Targets = mission.Targets.Select(enrichTarget).ToList();
statementData.TotalBonuses = totalBonuses(statementData);
statementData.TotalExperiences = totalExperiences(statementData);
return statementData;
}
}
After compilation and testing, this refactoring works. It's easy to create a rendering HTML format function and create a new unit testing function to test it. For usd function, it's transferred to top function because of common use:
public class MissionManager
{
public string Statement(HuntingMission mission, Dictionary<string, Monster> monsters)
{
...
}
public string HtmlStatement(HuntingMission mission, Dictionary<string, Monster> monsters)
{
return RenderHtml(new StatementDataManager().CreateStatementData(mission, monsters));
}
private string RenderHtml(StatementData data)
{
string result = $"<h1>Statement for {data.MissionName}</h1>\n";
result += $"<table>\n";
result += $"<tr><th>Monster Name</th><th>Bonus</th><th>Level</th></tr>";
foreach (var missionTarget in data.Targets)
{
result +=
$"<tr><td>{missionTarget.Monster.Name}</td><td>{Usd(missionTarget.Amount)}</td>";
result += $"<td>{missionTarget.Level}</td></tr>\n";
}
result += $"</table>\n";
result += $"<p>Bonus was <em>{Usd(data.TotalBonuses)}</em></p>\n";
result += $"<p>You got <em>{data.TotalExperiences}</em> experiences</p>\n";
return result;
}
private string RenderPlainText(StatementData data)
{
...
}
private string Usd(int aNumber)
{
CultureInfo enCulture = new CultureInfo("en-US");
Thread.CurrentThread.CurrentCulture = enCulture;
NumberFormatInfo localFormat = (NumberFormatInfo)NumberFormatInfo.CurrentInfo.Clone();
return (aNumber / 100).ToString("c", localFormat);
}
}
public class MIssionManagerTests
{
[Test]
public void Given_Mission_Monsters_Should_Return_Correct_Statement()
{
...
}
[Test]
public void Given_Mission_Monsters_Should_Return_Correct_HtmlStatement()
{
// arrange
Dictionary<string, Monster> monsters = new Dictionary<string, Monster>();
monsters.Add("fire_giant", new Monster("Fire Giant", "giant"));
monsters.Add("stone_giant", new Monster("Stone Giant", "giant"));
monsters.Add("bone_dragon", new Monster("Bone Dragon", "dragon"));
HuntingMission mission = new HuntingMission("Giant & Dragon Hunting", new List<HuntingTarget>(new[]
{
new HuntingTarget("fire_giant", 55),
new HuntingTarget("bone_dragon", 40),
new HuntingTarget("stone_giant", 61)
}));
List<HuntingMission> missions = new List<HuntingMission>(new[] { mission });
// act
string result = new MissionManager().HtmlStatement(missions.First(), monsters);
// assert
Assert.AreEqual(
"<h1>Statement for Giant & Dragon Hunting</h1>\n<table>\n<tr><th>Monster Name</th><th>Bonus</th><th>Level</th></tr><tr><td>Fire Giant</td><td>$750.00</td><td>55</td></tr>\n<tr><td>Bone Dragon</td><td>$950.00</td><td>40</td></tr>\n<tr><td>Stone Giant</td><td>$780.00</td><td>61</td></tr>\n</table>\n<p>Bonus was <em>$2,480.00</em></p>\n<p>You got <em>49</em> experiences</p>\n",
result);
}
}
After compilation and testing, this refactoring works.
Current files are separated into MissionManager and StatementDataManager 2 files. Although these code lines has increased, refactoring makes the code more readability. This separation of logic (calculation & rendering) enhances the modular design and no redundant calculation logic.
The tip at book's page 34 describes:
When programming, follow the camping rule: Always leave the code base healthier than when you found it.
Make the code healthier again!
In this example, amountFor and experienceFor functions determines their result based on the monster type. Current type has dragon and giant. But in the future the type possibly increases, and using if/else if to extend the feature is not a good choice. Replace Conditional with Polymorphism is a better skill to refactor it. Before use polymorphism, should create a class and it contains amountFor and experienceFor functions.
In StatementDataManager's enrichTarget function, it calls amountFor and experienceFor, So I create a class HuntingTargetCalculator and use this class to call the 2 functions:
internal class StatementDataManager
{
public StatementData CreateStatementData(HuntingMission mission, Dictionary<string, Monster> monsters)
{
Func<CalculatedTarget, Monster> monsterFor = delegate (CalculatedTarget missionTarget)
{
...
};
Func<CalculatedTarget, int> amountFor = delegate (CalculatedTarget missionTarget)
{
...
};
Func<CalculatedTarget, int> experienceFor = delegate (CalculatedTarget missionTarget)
{
...
};
Func<HuntingTarget, CalculatedTarget> enrichTarget = delegate (HuntingTarget target)
{
var calculator = new HuntingTargetCalculator(target);
CalculatedTarget result = new CalculatedTarget(target.MonsterId, target.Level);
result.Monster = monsterFor(result);
result.Amount = amountFor(result);
result.Experience = experienceFor(result);
return result;
};
Func<StatementData, int> totalExperiences = delegate (StatementData data)
{
...
};
Func<StatementData, int> totalBonuses = delegate (StatementData data)
{
...
};
StatementData statementData = new StatementData();
statementData.MissionName = mission.Name;
statementData.Targets = mission.Targets.Select(enrichTarget).ToList();
statementData.TotalBonuses = totalBonuses(statementData);
statementData.TotalExperiences = totalExperiences(statementData);
return statementData;
}
}
internal class HuntingTargetCalculator
{
private readonly HuntingTarget _target;
public HuntingTargetCalculator(HuntingTarget target)
{
_target = target;
}
}
Let's create first property Monster in HuntingTargetCalculator. This property
iThome鐵人賽
看影片追技術